types: add support RFC 110 expanded year format#127
types: add support RFC 110 expanded year format#127patjakdev merged 2 commits intocedar-policy:mainfrom
Conversation
d03baa3 to
327dd3b
Compare
types/datetime.go
Outdated
|
|
||
| if s[13] != ':' { | ||
| return Datetime{}, fmt.Errorf("%w: unexpected character %s", errDatetime, strconv.QuoteRune(rune(s[13]))) | ||
| if hour, s, err = parseTwoCharacterUint(s, 23, "hour"); err != nil { |
There was a problem hiding this comment.
All of this seems like there should be a single function that is just parseUint(s, expectedLen, maxVal, name)
And use that for all the components, not just for some of them.
|
|
||
| if s[10] != 'T' { | ||
| return Datetime{}, fmt.Errorf("%w: unexpected character %s", errDatetime, strconv.QuoteRune(rune(s[10]))) | ||
| if err = checkValidDay(year, month, day); err != nil { |
There was a problem hiding this comment.
Seems like you should throw in hour, minute, second as well, and use the other methods on date .Hour(), .Minute(), .Second() to validate that yes this is exactly what it is. (This should help in cases of leap seconds maybe?)
There was a problem hiding this comment.
This check is just about validating that the day is valid for the given month/year. The other values are validated by their maxValue, day is the only one whose max value depends on other values.
If you're talking about the checks for the minimum and maximum timestamp, I neglected to even do that checking in the original PR. Hopefully the code I've added makes that clear.
philhassey
left a comment
There was a problem hiding this comment.
A few suggestions and a few nits and a few tests might be worth addressing here.
I think my thoughts on validating h:m:s are probably not valid thoughts. But the other changes seemed reasonable.
3858eaf to
b6f3b6d
Compare
An offset of +0100 indicates that 1 hour should be _subtracted_ from the time in order to produce UTC rather than added. Signed-Off-By: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
f699039 to
b6460fd
Compare
This change adds support for the nine-digit expanded year datetime format specified in RFC 110, allowing exotic datetimes far in the past or future to be represented in Cedar policies and entities. Signed-Off-By: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
b6460fd to
c1177bf
Compare
Issue #, if available:
#128
Description of changes:
This change adds support for the nine-digit expanded year datetime format specified in RFC 110, allowing exotic datetimes far in the past or future to be represented in Cedar policies and entities.
Incidentally, it also fixes a bug in the
Datetimetype where timezone offset was being interpreted incorrectly (see #128).